-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Add feat/jsdoc-require-returns-type
eslint rule
#115
chore: Add feat/jsdoc-require-returns-type
eslint rule
#115
Conversation
🦋 Changeset detectedLatest commit: 5d17c47 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm approving based on the CI passing.
I still think it's counterintuitive to be doing ESLint types before we fix/enforce our TS types 🤷
Pinging @ayushnirwal in case he has something to add.
feat/jsdoc-require-returns-type
eslint rulefeat/jsdoc-require-returns-type
eslint rule
(its a |
(better to enforce return types vs TS - on hold) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use require-returns to enforce descriptions. (as a warning)
Use explicit-function-return-type to enforce return types in TS (as an error)
Hi @ayushnirwal, cc: @justlevine |
…maankhan/snapwp; branch 'develop' of github.com:rtCamp/snapwp into feat/jsdoc-require-returns-type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for me to review this again. Once @ayushnirwal approves this and a changeset is added, lmk and I'll merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is mergable after this and merge conflicts are resolved. Also, changeset too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, I switched the return types from the problematic JSX.Element
to ReactNode
.
Also explicitly importing types ... from 'React';
since it's better to be explicit.
What
Related Issue(s):
Testing Instructions
npm install
&&npm run lint
.Additional Info
eslint
errors.Checklist
npm run changeset
.